Skip to content

Conversation

@devshgraphicsprogramming
Copy link
Member

@keptsecret after you merge master again, you'll probably have the UI mess up and show changed from the merge commit, so close and reopen again

Comment on lines +11 to +14
list(APPEND NBL_LIBRARIES
imtestengine
"${NBL_EXT_IMGUI_UI_LIB}"
)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AnastaZIuk is that needed?

Comment on lines +16 to +109
namespace nbl
{
namespace hlsl
{
namespace ext
{

template<typename T> // TODO make type T Spectrum
struct Payload
{
using this_t = Payload<T>;
using scalar_type = T;
using vector3_type = vector<T, 3>;

vector3_type accumulation;
scalar_type otherTechniqueHeuristic;
vector3_type throughput;
// #ifdef KILL_DIFFUSE_SPECULAR_PATHS
// bool hasDiffuse;
// #endif
};

enum ProceduralShapeType : uint16_t
{
PST_NONE = 0,
PST_SPHERE,
PST_TRIANGLE,
PST_RECTANGLE
};

struct ObjectID
{
static ObjectID create(uint32_t id, uint32_t mode, ProceduralShapeType shapeType)
{
ObjectID retval;
retval.id = id;
retval.mode = mode;
retval.shapeType = shapeType;
return retval;
}

uint32_t id;
uint32_t mode;
ProceduralShapeType shapeType;
};

template<typename T>
struct Ray
{
using this_t = Ray<T>;
using scalar_type = T;
using vector3_type = vector<T, 3>;

// immutable
vector3_type origin;
vector3_type direction;

// polygon method == PPM_APPROX_PROJECTED_SOLID_ANGLE
vector3_type normalAtOrigin;
bool wasBSDFAtOrigin;

// mutable
scalar_type intersectionT;
ObjectID objectID;

Payload<T> payload;
};

template<class Spectrum>
struct Light
{
using spectral_type = Spectrum;

NBL_CONSTEXPR_STATIC_INLINE uint32_t INVALID_ID = 0xffffu;

static Light<spectral_type> create(NBL_CONST_REF_ARG(spectral_type) radiance, uint32_t objId, uint32_t mode, ProceduralShapeType shapeType)
{
Light<spectral_type> retval;
retval.radiance = radiance;
retval.objectID = ObjectID::create(objId, mode, shapeType);
return retval;
}

static Light<spectral_type> create(NBL_CONST_REF_ARG(spectral_type) radiance, NBL_CONST_REF_ARG(ObjectID) objectID)
{
Light<spectral_type> retval;
retval.radiance = radiance;
retval.objectID = objectID;
return retval;
}

spectral_type radiance;
ObjectID objectID;
};
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is just the application, it shouldn't be in any namespace

{
namespace ext
{
namespace PathTracer
Copy link
Member Author

@devshgraphicsprogramming devshgraphicsprogramming Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lower snake_case path_tracing we can also move it to core Nabla and not keep in ext

Comment on lines 25 to 41
template<typename BxDFCreation, typename Scalar>
struct PathTracerCreationParams
{
// rng gen
uint32_t2 rngState;

// ray gen
vector<Scalar, 2> pixOffsetParam;
vector<Scalar, 3> camPos;
vector<Scalar, 4> NDC;
matrix<Scalar, 4, 4> invMVP;

// mat
BxDFCreation diffuseParams;
BxDFCreation conductorParams;
BxDFCreation dielectricParams;
};
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this stuff is userspace it doesn't belong here

Comment on lines 43 to 44
template<class RandGen, class RayGen, class Intersector, class MaterialSystem, /* class PathGuider, */ class NextEventEstimator>
struct Unidirectional
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concepts for everything (but don't define them in this file)

Comment on lines +13 to +44
template<typename Light, typename BxdfNode>
struct Scene
{
using light_type = Light;
using bxdfnode_type = BxdfNode;
using this_t = Scene<Light, BxdfNode>;

// NBL_CONSTEXPR_STATIC_INLINE uint32_t maxSphereCount = 25;
// NBL_CONSTEXPR_STATIC_INLINE uint32_t maxTriangleCount = 12;
// NBL_CONSTEXPR_STATIC_INLINE uint32_t maxRectangleCount = 12;

#if SPHERE_COUNT < 1
#define SCENE_SPHERE_COUNT 1
#else
#define SCENE_SPHERE_COUNT SPHERE_COUNT
#endif

#if TRIANGLE_COUNT < 1
#define SCENE_TRIANGLE_COUNT 1
#else
#define SCENE_TRIANGLE_COUNT TRIANGLE_COUNT
#endif

#if RECTANGLE_COUNT < 1
#define SCENE_RECTANGLE_COUNT 1
#else
#define SCENE_RECTANGLE_COUNT RECTANGLE_COUNT
#endif

Shape<PST_SPHERE> spheres[SCENE_SPHERE_COUNT];
Shape<PST_TRIANGLE> triangles[SCENE_TRIANGLE_COUNT];
Shape<PST_RECTANGLE> rectangles[SCENE_RECTANGLE_COUNT];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to get rid of the #defines so make a SceneBase with the 6 balls which areCONSTEXPR_STATIC_INLINE and then each derived class only does the light (which is movable by push constant supplied matrix)

Comment on lines +6 to +11
namespace nbl
{
namespace hlsl
{
namespace ext
{
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

userspace, no namespace

Comment on lines +46 to +48
uint32_t sphereCount;
uint32_t triangleCount;
uint32_t rectangleCount;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these should be hardcoded constants (basically we have 3 different scenes

Comment on lines +57 to +58
bxdfnode_type bxdfs[BXDF_COUNT];
uint32_t bxdfCount;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that should sit in the MaterialSystem and shape can index into it

Comment on lines +50 to +53
// NBL_CONSTEXPR_STATIC_INLINE uint32_t maxLightCount = 4;

light_type lights[LIGHT_COUNT];
uint32_t lightCount;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that belongs in Next Event Estimator, shape can index into it

Comment on lines +62 to +84
static this_t create(
NBL_CONST_REF_ARG(Shape<PST_SPHERE>) spheres[SCENE_SPHERE_COUNT],
NBL_CONST_REF_ARG(Shape<PST_TRIANGLE>) triangles[SCENE_TRIANGLE_COUNT],
NBL_CONST_REF_ARG(Shape<PST_RECTANGLE>) rectangles[SCENE_RECTANGLE_COUNT],
uint32_t sphereCount, uint32_t triangleCount, uint32_t rectangleCount,
NBL_CONST_REF_ARG(light_type) lights[LIGHT_COUNT], uint32_t lightCount,
NBL_CONST_REF_ARG(bxdfnode_type) bxdfs[BXDF_COUNT], uint32_t bxdfCount)
{
this_t retval;
retval.spheres = spheres;
retval.triangles = triangles;
retval.rectangles = rectangles;
retval.sphereCount = sphereCount;
retval.triangleCount = triangleCount;
retval.rectangleCount = rectangleCount;

retval.lights = lights;
retval.lightCount = lightCount;

retval.bxdfs = bxdfs;
retval.bxdfCount = bxdfCount;
return retval;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we wont be needing this

# Showcase compute pathtracing
add_subdirectory(30_ComputeShaderPathTracer)

add_subdirectory(31_HLSLPathTracer EXCLUDE_FROM_ALL)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove EXCLUDE_FROM_ALL I want CI running on this before we merge

Comment on lines 16 to 19
[[vk::binding(1, 2)]] Buffer<uint3> sampleSequence;

[[vk::combinedImageSampler]][[vk::binding(2, 2)]] Texture2D<uint2> scramblebuf; // unused
[[vk::combinedImageSampler]][[vk::binding(2, 2)]] SamplerState scrambleSampler;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sampleSequence should just be a BDA provided via push constant of QuantizedSequence (3 21bit unorms packed into a struct of size 8 and alignment 4)

Comment on lines +7 to +8
int sampleCount;
int depth;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you add the BDA for sample sequence then you'll have just enough Push Constants left to add a float32_t3x4 for the light transform


[[vk::binding(1, 2)]] Buffer<uint3> sampleSequence;

[[vk::combinedImageSampler]][[vk::binding(2, 2)]] Texture2D<uint2> scramblebuf; // unused
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use the scramble buffer, it will allow us to do stuff like key-exchanges to attempt to get Blue Noise (as per some Heitz papers)

Comment on lines 19 to 23
struct PTPushConstant {
matrix4SIMD invMVP;
int sampleCount;
int depth;
};
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use the same struct as in your render_common.hlsl

using namespace nbl;
using namespace hlsl;

NBL_CONSTEXPR uint32_t WorkgroupSize = 512;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optimal is 64 due to not holding up everything with divergence (lots of small WGs in the same SM co-reisdent)

btw this needs to be in shared header

Comment on lines +45 to +46
NBL_CONSTEXPR uint32_t MAX_DEPTH_LOG2 = 4;
NBL_CONSTEXPR uint32_t MAX_SAMPLES_LOG2 = 10;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shared header!

Comment on lines +17 to +36
#ifdef SPHERE_LIGHT
#define SPHERE_COUNT 9
#define TRIANGLE_COUNT 0
#define RECTANGLE_COUNT 0
#endif

#ifdef TRIANGLE_LIGHT
#define TRIANGLE_COUNT 1
#define SPHERE_COUNT 8
#define RECTANGLE_COUNT 0
#endif

#ifdef RECTANGLE_LIGHT
#define RECTANGLE_COUNT 1
#define SPHERE_COUNT 8
#define TRIANGLE_COUNT 0
#endif

#define LIGHT_COUNT 1
#define BXDF_COUNT 7
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

want this baked in STATIC_INLINE_CONSTEXPR in 3 different scenes

Comment on lines +49 to +58
constexpr static inline uint32_t2 WindowDimensions = { 1280, 720 };
constexpr static inline uint32_t MaxFramesInFlight = 5;
constexpr static inline clock_t::duration DisplayImageDuration = std::chrono::milliseconds(900);
constexpr static inline uint32_t DefaultWorkGroupSize = 512u;
constexpr static inline uint32_t MaxDescriptorCount = 256u;
constexpr static inline uint32_t MaxDepthLog2 = 4u; // 5
constexpr static inline uint32_t MaxSamplesLog2 = 10u; // 18
constexpr static inline uint32_t MaxBufferDimensions = 3u << MaxDepthLog2;
constexpr static inline uint32_t MaxBufferSamples = 1u << MaxSamplesLog2;
constexpr static inline uint8_t MaxUITextureCount = 1u;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lot of this stuff should be in shared header between HLSL and C++

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants